Add cluster_hist_index argument for EDM4hep reconstructed particles#229
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #229 +/- ##
==========================================
+ Coverage 82.10% 82.67% +0.57%
==========================================
Files 21 21
Lines 1403 1403
==========================================
+ Hits 1152 1160 +8
+ Misses 251 243 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I was wondering, would it be possible to test the EDM4hep extension in the test suite? How heavy are the dependencies? I think we already had some issues with EDM4hep extension de-synchronizing slightly in the past |
|
I just checked the dependencies for addressing this in #230 and they are not terrible: EDM4hep depends on UnROOT depends on XRootD. |
m-fila
left a comment
m-fila
left a comment
There was a problem hiding this comment.
Seems fine, checked with some example files 👍
It is necessary for construction of EEJets from ::ReconstructedParticle that the cluster_hist_index is used.
Use a EDM4hep ReconstructedParticle (made "by hand") to check that our jets can be made from this type of input.
graeme-a-stewart
force-pushed
the
edm4hep-fixes
branch
from
6daccb8 to
14875a6
Compare
|
Uh oh! There is a failure here against Julia nightly, but it's due to an issue in the XRootD package that crops up. |
|
I checked against Julia nightly on OS X and Linux x86_64 and I get the same problems. The minimum way to reproduce it to try to add the (edm4hep) pkg> add XRootD
Resolving package versions...
Updating `~/tmp/edm4hep/Project.toml`
[164e3b87] + XRootD v0.2.3
Updating `~/tmp/edm4hep/Manifest.toml`
[1f15a43c] + CxxWrap v0.17.5
[692b3bcd] + JLLWrappers v1.7.1
[1914dd2f] + MacroTools v0.5.16
[21216c6a] + Preferences v1.5.1
[164e3b87] + XRootD v0.2.3
[9cdfc4e7] + JSON_C_jll v0.18.0+0
[94ce4f54] + Libiconv_jll v1.18.0+0
[38a345b3] + Libuuid_jll v2.41.2+0
⌅ [02c8fc9c] + XML2_jll v2.14.4+0
[bb14ce0c] + XRootD_cxxwrap_jll v0.3.0+0
[b6113df7] + XRootD_jll v5.8.4+0
[3eaa8342] + libcxxwrap_julia_jll v0.14.8+0
[5ef642bb] + libxcrypt_legacy_jll v4.5.2+0
[56f22d72] + Artifacts v1.11.0
[ade2ca70] + Dates v1.11.0
[8f399da3] + Libdl v1.11.0
[de0858da] + Printf v1.11.0
[fa267f1f] + TOML v1.0.3
[4ec0a83e] + Unicode v1.11.0
[e66e0078] + CompilerSupportLibraries_jll v1.3.0+1
[458c3c95] + OpenSSL_jll v3.5.5+0
[83775a58] + Zlib_jll v1.3.1+2
Info Packages marked with ⌅ have new versions available but compatibility constraints restrict them from upgrading. To see why use `status --outdated -m`
ERROR: LoadError: UndefVarError: `XRootD_exports` not defined in `XRootD_cxxwrap_jll`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
[1] getproperty(x::Module, f::Symbol)
@ Base ./Base_compiler.jl:50
[2] top-level scope
@ ~/.julia/packages/XRootD/Pmqyi/src/XRootD.jl:17
[3] include(mod::Module, _path::String)
@ Base ./Base.jl:309
[4] include_package_for_output(pkg::Base.PkgId, input::String, syntax_version::VersionNumber, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
@ Base ./loading.jl:3288
[5] top-level scope
@ stdin:5
[6] eval(m::Module, e::Any)
@ Core ./boot.jl:489
[7] include_string(mapexpr::typeof(identity), mod::Module, code::String, filename::String)
@ Base ./loading.jl:3130
[8] include_string
@ ./loading.jl:3140 [inlined]
[9] exec_options(opts::Base.JLOptions)
@ Base ./client.jl:342
[10] _start()
@ Base ./client.jl:585
in expression starting at /Users/graemes/.julia/packages/XRootD/Pmqyi/src/XRootD.jl:1
in expression starting at stdin:5
✗ XRootDI am rather flummoxed! @peremato any ideas? |
|
OK, I think to merge this PR as the nightly failure is not critical right now (just annoying!). I opened an issue against the Julia XRootD wrapper: JuliaHEP/XRootD.jl#3. |
2 participants
It is necessary for construction of EEJets from
::ReconstructedParticlethat thecluster_hist_indexis an optional argument so that the calls from the jet preprocessor (setting the correctcluster_hist_index) work.This has been checked against the examples in
examples/EDM4hep, both of which now work.A basic test case has been introduced that tests that EEJet and PseudoJet can be constructed from an
EDM4hep.edm4hep!ReconstructedParticle. This requires adding EDM4hep to thetestenvironment.Closes #228
Closes #230